test(dev-server): support UI port#1273
Conversation
temporalio/testing/_workflow.py
Outdated
| ui_port_str = str(dev_server_ui_port) | ||
| extra_args_list = list(dev_server_extra_args) | ||
| if "--ui-port" not in extra_args_list: | ||
| # Prepend to extra_args so user-provided args can override if needed |
There was a problem hiding this comment.
This comment is odd. You explicitly only even append the argument if it won't be overridden. I would either remove the comment or allow the argument to always be appended
There was a problem hiding this comment.
The PR approach should change IMO. The Rust side supports ui_port on TemporalDevServerConfig, this should be plumbed through the bridge. Extra args should not be touched. And arguably, we shouldn't prefix this option with dev_server_. Can look at .NET or Ruby for inspiration.
There was a problem hiding this comment.
@tconley1428 @cretz thanks for the review. I’ve taken a look at the Ruby implementation and am leveraging the Rust bridge. Next time, I’ll ask for more guidance before implementing the PR. Sorry about that.
tconley1428
left a comment
tconley1428
left a comment
There was a problem hiding this comment.
There are a number of linting errors which would need to be fixed.
Lujeni
force-pushed
the
feat/748
branch
3 times, most recently
from
e28a190 to
6478044
Compare
Lujeni
force-pushed
the
feat/748
branch
2 times, most recently
from
6478044 to
9adadc1
Compare
3 participants
What was changed
Added
ui_portparameter toWorkflowEnvironment.start_local()method to allow users to customize the UI port when starting a local Temporal dev server environment.Why?
This feature allows users to specify a custom UI port when starting the dev server
Checklist
Closes [Feature Request] Allow customization of dev server UI port #748
How was this tested:
Any docs updates needed?
start_local()method